Skip to content

fix(agent): stop re-appending repaired rows and saving the length nudge as user text - #120862

Open
jonpol01 wants to merge 1 commit into
NousResearch:mainfrom
jonpol01:fix/repaired-user-row-not-persisted-twice
Open

jonpol01 wants to merge 1 commit into
NousResearch:mainfrom
jonpol01:fix/repaired-user-row-not-persisted-twice

Conversation

@jonpol01

@jonpol01 jonpol01 commented Sep 24, 2026 •

Copy link
Copy Markdown

What does this PR do?

A repaired user row is saved once, and Hermes's internal length-continuation nudge is no longer saved as the user's own words.

Before this change, #117569 popped a saved row's marker so that "the next flush rewrites it". But the flush only appends, so the user's prompt was inserted a second time and every later turn replayed both. The thinking-only length retry and the interrupted lease-wait carry both hit it.

Related Issue

Fixes #120861

This fixes forward from #117569 (#117120, salvage of #117121) and keeps its intent: an in-place repair of a persisted dict reaches state.db, now as an in-place rewrite. Nothing is reverted.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • A. agent/transcript_repair.py: a message that already owns an active row is never inserted again. Dropped in the 2026-09-28 rebase: main's rewrite of transcript_repair.py (row-snapshot compare-and-swap, same-role clone lookup) already resolves a _row_id dict to its row and never re-inserts it. The branch now carries only B and C, and both tests below are still red on main (checked after the rebase).

  • B. agent/turn_truncation.py, session_persistence.py, turn_final_response.py: a length nudge that would follow this turn's already-saved user row is flagged _length_continuation_synthetic. That makes it request-only ephemeral scaffolding, like _dropped_toolcall_nudge: the wire copy still merges it for alternation, but it is never saved or merged into the real row, and it's removed before the final answer.

    • Skipping the nudge outright would break test_empty_stub_only_appends_continuation_user_message, which requires the chunking nudge to reach the model.
  • C. agent/session_persistence.py: _PERSIST_AFTER_ADMISSION_INTERRUPT is cleared after its one write, so the carried row stops bypassing the history skip.

  • agent/agent_runtime_helpers.py _merge_consecutive_users never merges ephemeral scaffolding into a real user row.

  • Tests (tests/agent/test_session_persistence_repaired_user_row.py) drive the real turn loop with a fake LLM client and a real SessionDB. No request leaves the machine.

    • test_thinking_only_length_retry_saves_the_prompt_once_and_never_the_nudge
    • test_prompt_carried_past_an_interrupted_lease_wait_is_saved_once: a real held lease, the status callback, and interrupt().
    • Both are red on main, with the saved prompt duplicated and the nudge saved as user text. Reverting each piece of B and C on its own reddens a test.
    • Piece A was checked outside these two tests with two real-loop runs:

    Happy to add either as a third test if wanted.

Trade-off: after such a continuation, the next turn replays the prompt without the nudge, so the prompt cache misses from that message onward, exactly as _dropped_toolcall_nudge already does. Before, the corrupted merged text was what got cached.

Open PRs:

How to Test

  1. scripts/run_tests.sh over test_session_persistence_repaired_user_row.py and the fix(agent): in-place repairs of persisted message dicts reach state.db instead of leaving stale rows (#117120, salvage #117121) #117569 suites (test_message_sequence_repair, test_micro_compaction, test_tool_call_args_sanitizer, test_unicode_ascii_codec), plus test_partial_stream_finish_reason: 143 passed.
  2. 78 related files over continuation, lease, batch, scaffolding, repair and _row_id: 1918 passed.
    • The two failures in test_run_agent.py and one in tui_gateway fail identically on unmodified main on this host: the optional anthropic SDK is missing, and a git-probe fork segfaults.
  3. Hunt repro: the saved transcript after turn 2 is [user 'first question', assistant, user 'write me a long report', assistant], with the prompt once and no nudge row.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass. I ran only the related files above through scripts/run_tests.sh, not the full suite.
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Sep 24, 2026
…ge as user text

a48b4c7 (NousResearch#117569) pops _db_persisted when a repair rewrites an already-written dict, on
the premise that the next flush rewrites the row. The flush only appends (except blank
assistant rows), so the repaired row was INSERTed a second time:

- Thinking-only length truncation: the continuation nudge lands right after this turn's
  written user row; the pre-request repair merges it in and pops the marker, so state.db
  holds the prompt twice, the second copy carrying "[System: Your previous response was
  truncated ...]" as the user's own words, replayed on every later turn.
- A prompt carried past an interrupted turn-lease wait keeps
  _persist_after_admission_interrupt forever, so the follow-up turn's merge into it bypasses
  the history skip and is appended again.
- Surrogate recovery rewriting an already-flushed tool result appends a second result for
  the same tool_call_id.

Fix forward, keeping NousResearch#117120's intent (in-place repairs reach state.db):

- transcript_repair: a dict whose _row_id owns an active row of its role (or that row's
  watermark clone) is rewritten in place (content, api_content), never inserted; assistant
  rows keep the blank-update / concurrent-winner rules.
- turn_truncation: a nudge that would follow a user row is request-only scaffolding
  (_length_continuation_synthetic): never persisted, never merged into the real row, popped
  before the final answer. The wire copy still merges it, so the request is unchanged.
- session_persistence: the admission-interrupt flag is cleared once the carried row is
  written.
@jonpol01
jonpol01 force-pushed the fix/repaired-user-row-not-persisted-twice branch from f3eab29 to 8896796 Compare September 28, 2026 03:39

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: a repaired user row is saved to state.db twice, and the length nudge is saved as the user's words (regression from #117569)

2 participants